Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix parallel mode under Python-3.8 #32

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

ploxiln
Copy link
Owner

@ploxiln ploxiln commented Mar 20, 2020

attempt to fix #27
(but probably only works for basic cases)

@ploxiln ploxiln added the wip work-in-progress label Mar 20, 2020
@ploxiln
Copy link
Owner Author

ploxiln commented Mar 30, 2020

Probably a better idea: add a thread mode as an alternative to the multiprocessing mode for parallel execution. env would be a thread-local variable. Most typical tasks would work with parallel-thread mode, but it would not be perfectly compatible with all existing tasks, which may depend on other user global state variables not being shared between parallel tasks, so thread-mode and multiprocessing-mode would both be options.

@maxmalysh
Copy link

I've tried

pip install https://github.com/ploxiln/fab-classic/archive/python38_parallel.tar.gz

with this fabfile:

from fabric.api import env, put, run, parallel

env.hosts = '[email protected]'

@parallel
def test():
    run('hostname')

and this is what I got:

(venv) MacBook-Pro:myproject mmalysh$ fab test
[[email protected]] Executing task 'test'
!!! Parallel execution exception under host '[email protected]':
Process [email protected]:
Traceback (most recent call last):
  File "/Users/mmalysh/.pyenv/versions/3.8.2/lib/python3.8/multiprocessing/process.py", line 315, in _bootstrap
    self.run()
  File "/Users/mmalysh/.pyenv/versions/3.8.2/lib/python3.8/multiprocessing/process.py", line 108, in run
    self._target(*self._args, **self._kwargs)
  File "/Users/mmalysh/Development/myproject/venv/lib/python3.8/site-packages/fabric/tasks.py", line 220, in _parallel_wrap
    queue.put({'name': name, 'result': task.run(*args, **kwargs)})
AttributeError: 'function' object has no attribute 'run'

Fatal error: One or more hosts failed while executing task 'test'

Underlying exception:
    'function' object has no attribute 'run'

Aborting.

@ploxiln
Copy link
Owner Author

ploxiln commented Apr 3, 2020

Yeah, this assumes that you decorate your task functions with @task

@maxmalysh
Copy link

It kind of works...

@task
@parallel
def test():
    run('hostname')

Output:

(venv) MacBook-Pro:myproject mmalysh$ fab test
[[email protected]] Executing task 'test'
[[email protected]] Executing task 'test'
[[email protected]] run: hostname
[[email protected]] run: hostname
[[email protected]] out: mesg: ttyname failed: Inappropriate ioctl for device
[[email protected]] out: sun.foobar.com
[[email protected]] out: 

[[email protected]] out: mesg: ttyname failed: Inappropriate ioctl for device
[[email protected]] out: moon.foobar.com
[[email protected]] out: 


Done.

Notice the error: mesg: ttyname failed: Inappropriate ioctl for device.

ploxiln added 2 commits April 5, 2020 02:18
leave it for parallel mode to be able to re-import tasks
because python-3.8 will not pickle the task, because:

    Can't pickle <function shellcmd at 0x10312c9d0>: it's not the same object as fabfile.shellcmd
@ploxiln ploxiln force-pushed the python38_parallel branch from 0cc521e to c95dca8 Compare April 5, 2020 06:19
@gingerlime
Copy link

gingerlime commented Apr 23, 2020

I'm seeing the error as well (upgrading from fabric 1.4 to fab-classic 1.18 with python 2.x)

[[email protected]] out: mesg: ttyname failed: Inappropriate ioctl for device

@ploxiln
Copy link
Owner Author

ploxiln commented Apr 23, 2020

that's not related to this PR or the issue this PR attempts to solve (parallel under python-3.8). that only is related to getting a PTY

@gingerlime
Copy link

gingerlime commented Apr 23, 2020

Sorry just searched for the error and that’s the first thing that popped up so I thought it’s somehow related. Happy to open a separate issue

EDIT: reported #46

@rafecolton
Copy link

Any update on this? We are seeing the same issue.

@Stealthii
Copy link

Stealthii commented May 5, 2022

Unfortunately this still struggles when running under parallel mode (in 3.8+), two problems being the with_settings decorator not applying to the tasks execute wraps, and more importantly, execute does not seem wait on completion:

File ~/host_finder.py:348, in HostDataCollector.host_report(self, datacenter)
    345 output = execute(get_kvm_data, roles=[datacenter])
    347 # Trim trash results, and log a warning why this happened
--> 348 for key, value in output.items():
    349     if not isinstance(value, collections.abc.MutableMapping):
    350         logger.warning("%s raised when retrieving data from %s", value, key)

RuntimeError: dictionary changed size during iteration

Edit: The above test was run on Python 3.8.6. I'd considered this might be related to python/cpython#83541, but this still occurs under Python 3.8.13.

@Stealthii
Copy link

Stealthii commented May 5, 2022

Ignore the above - I incorrectly attributed this to dict updates after execute finished, this is not a bug in fab-classic, this was me being stupid when putting together a test.

However, there are still behavioural changes with 3.8 compared to 3.6/3.7 - one is around env context not getting applied down the chain (which explains env settings not applying even within the same method, and @with_settings decorator on the method calling execute not applying to tasks run underneath. @ploxiln I know you addressed this in your first comment on here.

Execution time of parallel jobs is also significantly higher - on the above test against 68 hosts (with parallel pool size set to 100), Python 3.7 will take ~8s to finish execution, with Python 3.8 taking ~45s for the same task.

@Stealthii
Copy link

I've had great success with ensuring the default processing call uses fork() and not spawn() - pre 3.8 speed, behaviours and execution speed on macOS seem to be upheld:

diff --git a/fabric/tasks.py b/fabric/tasks.py
index 47a2b49f..dcb34d5f 100644
--- a/fabric/tasks.py
+++ b/fabric/tasks.py
@@ -329,6 +329,7 @@ def execute(task, *args, **kwargs):
         # if it can't.
         try:
             import multiprocessing
+            multiprocessing.set_start_method('fork')
         except ImportError:
             import traceback
             tb = traceback.format_exc()

@ploxiln it might be worth making this change for non-Windows (or all if we don't maintain Windows support), until such a time as the multi-thread method can be rewritten as you suggest.

@ploxiln
Copy link
Owner Author

ploxiln commented May 6, 2022

I'm pretty sure parallel mode never worked with windows anyway, so we could just switch to "fork" method now.

@boblozano
Copy link

Just came across your work, 1000% thank you for forking fab-classic.

As for this issue, after dealing with so many old / fragile py2 related dependencies, really like the looks of this ability to restore parallel tasks in current python. Mg.

@ajakubo1
Copy link

ajakubo1 commented Oct 5, 2023

I know this is not a proper fix or anything, but one can use this workaround without doing any monkey-patching:

In my main fabfile I did this:

import multiprocessing
multiprocessing.set_start_method('fork')

And it seems to be working nicely.

I just realized this as I was stuck to python3.7 earlier and I'm trying to do an upgrade now.

Also @ploxiln - based on the docs for multiprocessing: https://docs.python.org/3/library/multiprocessing.html

Changed in version 3.8: On macOS, the spawn start method is now the default. The fork start method should be considered unsafe as it can lead to crashes of the subprocess as macOS system libraries may start threads. See bpo-33725.

I believe that this is the reason why the problem started happening for me. As in - it would have also started for earlier python versions if they would use spawn (though I didn't check that part myself, just trusted the docs). If that's the case, I think it's OK to use it with fabric (since this is what I was doing earlier without even realizing it).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wip work-in-progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

@parallel doesn't work with Python 3.8
7 participants